packages/lib/server/service/insightsBooking.ts (3)

29-34: InsightsBookingServicePublicOptions is looser than the Zod schema.

The public type makes teamId optional for all scopes, but the Zod schema requires it for "team" scope. While safeParse handles validation at runtime, callers using this type won't get compile-time enforcement that teamId must be provided when scope === "team". Consider using a discriminated union type matching the schema, or at least adding a doc comment noting the Zod validation requirement.

♻️ Suggested type that mirrors the schema

-export type InsightsBookingServicePublicOptions = {
-  scope: "user" | "org" | "team";
-  userId: number;
-  orgId: number;
-  teamId?: number;
-};
+export type InsightsBookingServicePublicOptions =
+  | { scope: "user"; userId: number; orgId: number }
+  | { scope: "org"; userId: number; orgId: number }
+  | { scope: "team"; userId: number; orgId: number; teamId: number };

---

68-81: Dead code: authConditions is always truthy, making two branches unreachable.

getAuthorizationConditions() always returns a Prisma.Sql object (either real conditions or NOTHING_CONDITION). A Prisma.Sql instance is always truthy, so the else if (filterConditions) branch (line 76) and the else branch (line 78-80) are unreachable.

♻️ Simplified logic

   async getBaseConditions(): Promise<Prisma.Sql> {
     const authConditions = await this.getAuthorizationConditions();
     const filterConditions = await this.getFilterConditions();
 
-    if (authConditions && filterConditions) {
+    if (filterConditions) {
       return Prisma.sql`(${authConditions}) AND (${filterConditions})`;
-    } else if (authConditions) {
+    } else {
       return authConditions;
-    } else if (filterConditions) {
-      return filterConditions;
-    } else {
-      return NOTHING_CONDITION;
     }
   }

---

104-112: Falsy checks on filter IDs will silently skip 0 values.

if (this.filters.eventTypeId) and if (this.filters.memberUserId) use truthiness, so an ID of 0 would be ignored. While auto-increment IDs are typically positive, using explicit !== undefined (or != null) would be more defensive and express intent more clearly.

♻️ Proposed fix

-    if (this.filters.eventTypeId) {
+    if (this.filters.eventTypeId != null) {
       conditions.push(
         Prisma.sql`("eventTypeId" = ${this.filters.eventTypeId}) OR ("eventParentId" = ${this.filters.eventTypeId})`
       );
     }
 
-    if (this.filters.memberUserId) {
+    if (this.filters.memberUserId != null) {
       conditions.push(Prisma.sql`"userId" = ${this.filters.memberUserId}`);
     }

packages/lib/server/service/__tests__/insightsBooking.integration-test.ts (1)

332-343: Org scope assertion depends on query result ordering, which may be non-deterministic.

The toEqual check on the user IDs array ([testData.user.id, user2.id, user3.id]) assumes MembershipRepository.findAllByTeamIds returns users in a specific order. If the repository doesn't guarantee ordering, this test could be flaky. Since auto-increment IDs are assigned in creation order and the DB likely returns them sorted, this works in practice, but it's inherently fragile.

Consider sorting the IDs in the service's buildOrgAuthorizationCondition before constructing the SQL, which would make both the query and the test deterministic.